Skip to content

Adding final to public API is API-stable. #32074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2020

Conversation

jckarter
Copy link
Contributor

@jckarter jckarter commented May 28, 2020

Client code can't override or subclass a public declaration already, so although the ABI differs, the API is the same whether something is final or not (unless the library makes additional, independently API-breaking changes, such as revoking open-ness or hiding public subclasses).

Client code can't override or subclass a `public` declaration already, so although the ABI differs, the API is the same
whether something is `final` or not.
@jckarter jckarter requested a review from nkcsgexi May 28, 2020 21:27
@jckarter
Copy link
Contributor Author

Hm, final should be APIBreakingToRemove, though, because making a final class non-final would break retroactive protocol conformances or other code that might rely on the class's invariance. I'll fix that too.

@jckarter
Copy link
Contributor Author

@slavapestov points out the API-breaking-ness of removing final only really applies to classes. So it might need special handling to be completely accurate here.

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b89ef78

@jckarter
Copy link
Contributor Author

@swift-ci Please test OS X

@jckarter jckarter merged commit b89ef78 into swiftlang:master May 30, 2020
@jckarter
Copy link
Contributor Author

This hitched a ride with #32088

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants